Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Return gtmPlugin from app context #409

Merged
merged 4 commits into from
May 27, 2024
Merged

Return gtmPlugin from app context #409

merged 4 commits into from
May 27, 2024

Conversation

filiphazardous
Copy link
Contributor

I have a problem with Vite inline, which in production mode just copies the useGtm function wholesale to the calling file, without copying the scope. This fix solves that problem, since it always fetches the gtm instance from the vue app context

@filiphazardous filiphazardous mentioned this pull request Feb 7, 2024
Open
@filiphazardous
Copy link
Contributor Author

Here's the bug in nuxt-gtm that this would solve (at least in my use-case): zadigetvoltaire/nuxt-gtm#14

@Shinigami92
Copy link
Contributor

Looks like getCurrentInstance is there since Vue v3.0.0. So I guess this is safe.

vuejs/core@9c0f820

Shinigami92
Shinigami92 reviewed Feb 7, 2024
View reviewed changes
src/index.ts Outdated Show resolved Hide resolved
src/index.ts Outdated Show resolved Hide resolved
@Shinigami92 Shinigami92 changed the title Fix: return gtmPlugin from app context Fix return gtmPlugin from app context Feb 7, 2024
@Shinigami92
Copy link
Contributor

Is it possible to add a test for this? So we can prevent regression?

@filiphazardous
Copy link
Contributor Author

Is it possible to add a test for this? So we can prevent regression?

I'll see if I can come up with a way to test it. What triggered it in our case was two levels of indirection (it was initialized through nuxt-gtm and useGtm was called directly in our project, so I guess Vite couldn't connect the dots). I'll see if I can reproduce a similar sandbox.

@Shinigami92
Copy link
Contributor

Is it possible to add a test for this? So we can prevent regression?

I'll see if I can come up with a way to test it. What triggered it in our case was two levels of indirection (it was initialized through nuxt-gtm and useGtm was called directly in our project, so I guess Vite couldn't connect the dots). I'll see if I can reproduce a similar sandbox.

Maybe spy or mocks can help to simplify it

@Shinigami92 Shinigami92 changed the title Fix return gtmPlugin from app context Return gtmPlugin from app context May 27, 2024
@Shinigami92 Shinigami92 merged commit aac8479 into gtm-support:main May 27, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Shinigami92 Shinigami92 Shinigami92 approved these changes

Assignees

@filiphazardous filiphazardous

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@filiphazardous @Shinigami92